Skip to content

When a URDF link has multiple visual tags, use appropriate material for each #1329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Open

When a URDF link has multiple visual tags, use appropriate material for each #1329

wants to merge 1 commit into from

Conversation

ANogin
Copy link

@ANogin ANogin commented Jan 22, 2025

Currently, when loading a URDF robot, rviz uses the same material for each link, even when the link has multiple <visual> tags with different materials. See #1293 (comment) for a detailed analysis.

Fixes #1293.

I tested that this fixes my use case on ROS Iron (display of a robot inside MoveIt plugin), but do not know enough to test more broadly. The code change appears to apply cleanly to rolling, but I have not tested outside of Iron either. Leaving it to maintainer to figure out the right branch(es) to merge this into.

@ANogin ANogin requested a review from ahcorde as a code owner January 22, 2025 05:06
@ANogin ANogin mentioned this pull request Jan 22, 2025
Open
@ANogin
Copy link
Author

ANogin commented Jan 22, 2025

I tried applying the same change to the jazzy branch of rviz, and testing with Jazzy. Everything seems to work and look correctly for my use case, but at the same time I am getting a bunch of

[ERROR] [1737532284.422022713] [rviz2]: Can't assign material Shape122Material to the ManualObject MeshShape_ManualObject0 because this Material does not exist in group General. Have you forgotten to define it in a .material script?
[ERROR] [1737532284.426383440] [rviz2]: Can't assign material Shape123Material to the ManualObject MeshShape_ManualObject1 because this Material does not exist in group General. Have you forgotten to define it in a .material script?
[ERROR] [1737532284.430191205] [rviz2]: Can't assign material Shape124Material to the ManualObject MeshShape_ManualObject2 because this Material does not exist in group General. Have you forgotten to define it in a .material script?

type messages. I am guessing this is not necessarily related to my changes, and is likely to be related to #1220 instead, but I am not sure...

ahcorde
ahcorde requested changes Jan 31, 2025
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iron is EOL, do you mind to target rolling ?

@ANogin ANogin force-pushed the fix_multiple_visuals_material branch from c5bc556 to de547fb Compare February 25, 2025 02:34
@ANogin ANogin changed the base branch from iron to rolling February 25, 2025 02:35
@ANogin
Copy link
Author

ANogin commented Feb 25, 2025 •
edited
Loading

iron is EOL, do you mind to target rolling ?

Updated; seems to work for my use case in Rolling. Not sure about the build failure reported by github - does not seem to have anything to do with my changes, and it builds fine for me on my machine.

@ahcorde anything else needed from me at this point? Thanks!

@ANogin ANogin requested a review from ahcorde February 25, 2025 19:39
@ahcorde
Copy link
Contributor

ahcorde commented Apr 25, 2025

Pulls: #1329
Gist: https://gist.githubusercontent.com/ahcorde/c29e0b2fab1b5d53a29e3bffba068076/raw/d384bf18736e826742f49c88cd177b3a5d87a5ea/ros2.repos
BUILD args: --packages-above-and-dependencies rviz_default_plugins
TEST args: --packages-above rviz_default_plugins
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15784

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

ahcorde
ahcorde requested changes Apr 25, 2025
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review linters https://ci.ros2.org/job/ci_linux-aarch64/17469/

@Bitterisland6
Copy link

@ANogin friendly ping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde requested changes

Assignees

@ahcorde ahcorde

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

using multiple visual-tags in URDF, but the material-tag of the first one is used for both
3 participants
@ANogin @ahcorde @Bitterisland6